Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Columns prefix and suffix on wrapper #3502

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

promatik
Copy link
Contributor

@promatik promatik commented Feb 1, 2021

Related with #3435.

This adds ability to add prefix and suffix to each column wrapper.

@tabacitu
Copy link
Member

tabacitu commented Feb 1, 2021

Note to self: we also talked about adding a wrapper[separator] attribute. Details in #3435 (comment)

Copy link
Member

@tabacitu tabacitu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed a possible error. Also, this needs a complementary PR to the 4.2 docs, before we merge it.

src/resources/views/crud/columns/inc/wrapper_end.blade.php Outdated Show resolved Hide resolved
promatik and others added 2 commits September 9, 2021 12:12
@tabacitu tabacitu changed the base branch from master to main July 31, 2022 05:40
@tabacitu
Copy link
Member

@promatik time to merge / close this. Still think it's a good idea & implementation? If so, please ask @pxpm to test & merge.

@tabacitu
Copy link
Member

tabacitu commented Nov 7, 2022

Bump! @promatik do you still think this is worth doing?

@promatik
Copy link
Contributor Author

promatik commented Nov 21, 2022

I think this is pretty cool 🤷‍♂️

image

And it can only be achieved with this feature, or, by hacking into the getFieldAttribute() { return "¿$this->field?";


Another example;

image

[
    ...
    'wrapper' => [
        ...
        'prefix' => '¿',
        'suffix' => '?',
    ],
    'prefix' => '—',
    'suffix' => '!',
],

@promatik promatik assigned tabacitu and unassigned promatik Nov 21, 2022
@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@promatik
Copy link
Contributor Author

Quick note, this highlights an inconsistency between the place of the column prefix and suffix, inside or outside the wrapper.

image

In some fields column prefix/suffix are inside, others are outside.
In my opinion it should always be outside, and if the devs want the prefix/suffix inside, use the method introduced in this PR, inside wrapper 👌

But, that is a breaking change, so not for now 🤷‍♂️

@tabacitu
Copy link
Member

tabacitu commented Nov 21, 2022

Awesome! Ok great, let's have this:

  • @promatik please create a PR for the docs
  • @pxpm please review/test and merge if it has your green light too

@tabacitu tabacitu assigned pxpm and unassigned tabacitu Nov 21, 2022
promatik added a commit to Laravel-Backpack/docs that referenced this pull request Dec 4, 2022
@promatik
Copy link
Contributor Author

promatik commented Dec 4, 2022

Docs here Laravel-Backpack/docs#389 🙌

@promatik promatik removed their assignment Dec 4, 2022
@tabacitu
Copy link
Member

@promatik if you think this is still worth doing, please add v6 docs for it (instead of v5) then ask Pedro to review and merge.

@tabacitu tabacitu assigned promatik and unassigned pxpm Oct 11, 2023
@promatik
Copy link
Contributor Author

promatik commented Jan 7, 2024

@pxpm updated docs to v6 Laravel-Backpack/docs#389 👌

@promatik promatik assigned pxpm and unassigned promatik Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants